Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR documents the new vMCP feature that enables MCPRemoteProxy resources to be aggregated alongside MCPServer resources by VirtualMCPServer, providing a unified endpoint for both local container-based and remote MCP servers.
Key changes include:
- Updated vMCP documentation to describe support for both MCPServer and MCPRemoteProxy backend types
- Added configuration examples showing how to add remote proxies to MCPGroups
- Extended the remote MCP proxy guide with a dedicated section on vMCP integration
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/toolhive/guides-vmcp/intro.mdx | Updated overview and architecture diagram to show MCPRemoteProxy as a supported backend type alongside MCPServer |
| docs/toolhive/guides-vmcp/configuration.mdx | Added "Add backends to a group" section with YAML examples for both MCPServer and MCPRemoteProxy configurations |
| docs/toolhive/guides-k8s/remote-mcp-proxy.mdx | Added "Use with Virtual MCP Server" section explaining how to add remote proxies to MCPGroups and the benefits of vMCP aggregation |
| docs/toolhive/concepts/vmcp.mdx | Updated multi-server aggregation section to mention MCPRemoteProxy support and mixed backend scenarios |
| remoteURL: https://mcp.analytics.example.com | ||
| transport: streamable-http | ||
| port: 8080 | ||
| oidcConfig: |
There was a problem hiding this comment.
What does the oidcConfig here mean? Is it a token that vMCP sends?
There was a problem hiding this comment.
vMCP currently does not send the token to MCPRemoteProxy as of yet. I have added the note about the authentication gap between vMCP and MCPRemoteProxy
| - **Unified endpoint**: Clients connect to one vMCP URL instead of multiple | ||
| proxy endpoints | ||
| - **Centralized authentication**: vMCP handles client authentication; remote | ||
| proxies validate the forwarded tokens |
There was a problem hiding this comment.
I assume we can't auth to remote servers as of yet?
| namespace: toolhive-system | ||
| spec: | ||
| groupRef: dev-tools | ||
| remoteURL: https://mcp.analytics.example.com |
There was a problem hiding this comment.
could we use a real MCP server in the example?
|
@amirejaz Is this PR still active? Jakub left some questions which I think haven't been addressed? |
Address reviewer feedback: - Replace fictional "analytics-proxy" with "context7-proxy" (real MCP server) - Add section on authenticating to upstream servers using headerInjection - Clarify that oidcConfig validates incoming requests (not a token vMCP sends) - Remove misleading claim about "forwarded tokens" - Use kubernetes oidcConfig type for in-cluster vMCP communication - Add externalAuthConfigRef example for upstream authentication
Document the current limitation where vMCP can discover MCPRemoteProxy backends but cannot authenticate to them because: - MCPRemoteProxy requires oidcConfig to validate incoming requests - vMCP does not yet forward authentication tokens to backends Changes: - Add caution callouts explaining the limitation - Remove misleading complete examples that suggest full functionality - Simplify the "Use with Virtual MCP Server" section - Note that this will be addressed in a future release
|
@danbarr there is an authentication gap between vMVP and MCPRemoteProxy so I have updated the PR to reflect that and this is ready for review now. |
Description
Documents the new vMCP feature that enables MCPRemoteProxy resources to be grouped with MCPServer resources and aggregated by VirtualMCPServer into a single unified endpoint.
Changes include:
groupRefusage and benefits of vMCP aggregationType of change
Related issues/PRs
Screenshots
N/A - content updates only, no structural changes
Submitter checklist
Content and formatting
Navigation
N/A - No pages were added, deleted, renamed, or reordered. Only content updates to existing pages.
Reviewer checklist
Content